-
Notifications
You must be signed in to change notification settings - Fork 0
Improve swap-deps error handling for deleted directories #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When swap-deps encounters a missing directory path (e.g., a deleted Conductor workspace), it now provides a helpful error message that: - Clearly states which path doesn't exist - Explains common causes (outdated .swap-deps.yml, deleted repo) - Suggests fixes (update config or use --restore) This makes the tool more user-friendly when dealing with stale configuration files that reference non-existent paths. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
WalkthroughUpdated error messaging in lib/demo_scripts/gem_swapper.rb by replacing a terse raise with a constructed multi-line, contextual error message in validate_local_paths!. No changes to control flow, signatures, or other logic. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review - PR #47: Improve swap-deps error handling for deleted directoriesOverviewThis PR enhances the error message in the validate_local_paths! method to provide more helpful guidance when local gem paths don't exist. The change is focused, minimal, and valuable. Strengths
Minor Suggestions1. String Building PerformanceThe current implementation uses multiple string concatenations with +=. While this works fine for error messages (which are rarely constructed), consider using a heredoc for slightly better readability. This is purely stylistic and not blocking. 2. Test Update ConsiderationThe test at gem_swapper_spec.rb:256 checks for the error message pattern. The regex pattern should still match, but you may want to add assertions to verify the enhanced message content is present. SecurityNo security concerns. The change only affects error messaging and doesn't introduce any vulnerabilities. BugsNo bugs identified. PerformanceNo performance impact. Error messages are only constructed when validation fails. Test CoverageAdequate coverage exists at gem_swapper_spec.rb:243-259. The test validates the error is raised with appropriate messaging. VerdictAPPROVED - This is a solid quality-of-life improvement. The suggestions above are optional enhancements, not blockers. The PR is ready to merge as-is. Great work improving the developer experience! Review generated with Claude Code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/demo_scripts/gem_swapper.rb (1)
522-530: Excellent improvement to error messaging!The enhanced error message is clear, actionable, and user-friendly. It provides context about what went wrong and offers concrete solutions.
Consider using a heredoc for better readability and efficiency:
- error_msg = "Local path for #{gem_name} does not exist: #{path}\n\n" - error_msg += "This usually means:\n" - error_msg += " 1. The path in .swap-deps.yml is outdated\n" - error_msg += " 2. You moved or deleted the local repository\n\n" - error_msg += "To fix:\n" - error_msg += " - Update .swap-deps.yml with the correct path\n" - error_msg += ' - Or use --restore to restore original dependencies' + error_msg = <<~ERROR_MSG + Local path for #{gem_name} does not exist: #{path} + + This usually means: + 1. The path in .swap-deps.yml is outdated + 2. You moved or deleted the local repository + + To fix: + - Update .swap-deps.yml with the correct path + - Or use --restore to restore original dependencies + ERROR_MSG
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/demo_scripts/gem_swapper.rb(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/demo_scripts/gem_swapper.rb (1)
522-530: Excellent error message improvement!The enhanced error message provides clear context, common causes, and actionable fixes. This will significantly improve the user experience when encountering missing gem paths.
Optional: Consider using a heredoc for cleaner multi-line string construction:
- error_msg = "Local path for #{gem_name} does not exist: #{path}\n\n" - error_msg += "This usually means:\n" - error_msg += " 1. The path in .swap-deps.yml is outdated\n" - error_msg += " 2. You moved or deleted the local repository\n\n" - error_msg += "To fix:\n" - error_msg += " - Update .swap-deps.yml with the correct path\n" - error_msg += ' - Or use --restore to restore original dependencies' - - raise Error, error_msg + error_msg = <<~ERROR_MSG + Local path for #{gem_name} does not exist: #{path} + + This usually means: + 1. The path in .swap-deps.yml is outdated + 2. You moved or deleted the local repository + + To fix: + - Update .swap-deps.yml with the correct path + - Or use --restore to restore original dependencies + ERROR_MSG + + raise Error, error_msg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error handling in the swap-deps tool by providing more helpful error messages when local dependency paths are missing. Instead of a brief error message, users now receive detailed guidance on likely causes and actionable solutions.
- Enhanced error message formatting with multi-line explanations
- Added context about common causes (outdated paths, moved/deleted repositories)
- Provided clear remediation steps (update config or use --restore)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary
Changes
validate_local_paths!to provide a more informative error message that includes:Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit